You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
De-semver-majorified variant of #6881: Instead of changing the return values of .listeners() unconditionally, adds an optional parameter that indicates which behaviour is desidered.
For unwrapOnceListeners = true, the array returned by .listeners() will include the handler functions as passed to .once().
For unwrapOnceListeners = false (the default and the current behaviour), the array returned by listeners() will include the wrapper functions used by .once(), not the original functions that were passed in from userland. Use cases for this include temporarily disabling an event by removing all handlers and adding them back at a later point, where it would be undesirable to deal with the original event handlers.
addaleax
added
events
Issues and PRs related to the events subsystem / EventEmitter.
semver-minor
PRs that contain new features and should be released in the next minor version.
labels
Jun 6, 2016
I agree with @cjihrig. I consider the fact that listeners() currently returns the once wrappers to be a bug that simply needs to be fixed. I'm -1 on introducing a new API or argument like this.
Hm yeah, so did I until I realized that the API as it exists now has use cases that wouldn’t be covered by the change in #6881, see e.g. this comment. If you think relying on the _events property is okay in those cases, then yeah, I’ll go ahead and close this.
I'm not convinced that those are cases we should necessarily care about and there are plenty of users who already go directly to _events for edge-case kinds of things (as much as we should discourage that). If new APIs are necessary to support such cases in the future, then so be it, but for this, I think the correct behavior is pretty clear (at least to me it is :-) ...)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eventsIssues and PRs related to the events subsystem / EventEmitter.semver-minorPRs that contain new features and should be released in the next minor version.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
addaleax
added
events
Checklist
Affected core subsystem(s)
events
Description of change
De-semver-majorified variant of #6881: Instead of changing the return values of
.listeners()unconditionally, adds an optional parameter that indicates which behaviour is desidered.For
unwrapOnceListeners = true, the array returned by.listeners()will include the handler functions as passed to.once().For
unwrapOnceListeners = false(the default and the current behaviour), the array returned bylisteners()will include the wrapper functions used by.once(), not the original functions that were passed in from userland. Use cases for this include temporarily disabling an event by removing all handlers and adding them back at a later point, where it would be undesirable to deal with the original event handlers.Ref: #6873
Ref: #6881
CI: https://ci.nodejs.org/job/node-test-commit/3659/